Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bpo-17576: Strict __int__ and __index__ return types; operator.index always uses __index__ #13740

Conversation

mdickinson
Copy link
Member

@mdickinson mdickinson commented Jun 2, 2019

This PR:

  • turns the DeprecationWarnings introduced by @serhiy-storchaka in Python 3.4 for non-integer return values from __int__ and __index__ into TypeErrors
  • makes operator.index and PyNumber_Index always give the same return value as __index__. This fixes the inconsistency reported by @warsaw in bpo-17576.

Still working on doc updates and what's-new entries, but making the PR now so that others can review.

https://bugs.python.org/issue17576

@mdickinson
Copy link
Member Author

mdickinson commented Jun 2, 2019

This PR should also allow simplifications to many parts of the codebase that currently need to check what type they got back from a PyNumber_Index call.

@@ -367,11 +367,12 @@ def test_n(self):
# (PY_SSIZE_T_MIN ... PY_SSIZE_T_MAX)
self.assertRaises(TypeError, getargs_n, 3.14)
self.assertEqual(99, getargs_n(Index()))
self.assertEqual(0, getargs_n(IndexIntSubclass()))
self.assertEqual(getargs_n(IndexIntSubclass()), 99)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nitpick: inconsistent arg ordering. Will fix.

@@ -66,10 +66,10 @@ def __index__(self):
direct_index = my_int.__index__()
operator_index = operator.index(my_int)
self.assertEqual(direct_index, 8)
self.assertEqual(operator_index, 7)
self.assertEqual(operator_index, 8)
Copy link
Member Author

@mdickinson mdickinson Jun 2, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the key change: for a subclass of int, we take the __index__ result to be the true value instead of the integer's value.

Py_INCREF(item);
return item;
}

/* In general, use __index__ */
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nitpick: redundant comment. I'll remove it.

@mdickinson mdickinson added type-feature A feature request or enhancement type-bug An unexpected behavior, bug, or error labels Jun 2, 2019
@mdickinson
Copy link
Member Author

The test failure appears to be due to test_asyncio altering the test environment. It seems unlikely that it's related to the changes in this PR (but anything's possible).

@serhiy-storchaka
Copy link
Member

serhiy-storchaka commented Jun 3, 2019

Some of these warnings were added in 3.8. We cannot turn them into TypeError in the same version.

@tirkarthi
Copy link
Member

The test failure appears to be due to test_asyncio altering the test environment. It seems unlikely that it's related to the changes in this PR (but anything's possible).

Could you please try rebasing on master to see if it helps. One of the assertion errors in test_asyncio was fixed with #13754 .

@mdickinson
Copy link
Member Author

mdickinson commented Jun 3, 2019

Some of these warnings were added in 3.8.

Are you sure? Which ones? The PR removes 3 DeprecationWarning blocks: one in abstract.c and two in longobject.c, and it looks to me as though all three were introduced in 31a6554

@serhiy-storchaka
Copy link
Member

serhiy-storchaka commented Jun 3, 2019

The warning added in bpo-36048.

@mdickinson
Copy link
Member Author

The warning added in bpo-36048.

Ah yes, right.

Good point. Closing.

@mdickinson mdickinson closed this Jun 3, 2019
@mdickinson mdickinson deleted the make-int-and-index-return-types-strict branch June 3, 2019 10:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting core review type-bug An unexpected behavior, bug, or error type-feature A feature request or enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants